PR Review – Fix standalone doctor SEO audit for BOM noindex pages

Overall this is a well-structured fix with a clear regression test. The core BOM-trimming change is small and correct. The PR also bundles a large amount of new feature work (git freshness metadata, PowerShell example validation/execution, example media rendering) beyond the stated fix, so comments below address both.

✅ What looks good

• BOM fix is correct. TrimLeadingByteOrderMark() is clean, handles the empty-string edge case, and is applied consistently in both the success path and the DecoderFallbackException catch block of ReadFileAsUtf8().
• Regression test is solid. Writing the file with new UTF8Encoding(true) (BOM-emitting) and asserting no SEO issues are raised directly covers the reported defect.
• HTML encoding. New badge HTML in BuildFreshnessBadgeHtml() uses HtmlEncode / HtmlAttributeEncode consistently — no obvious XSS surface.
• Separation of concerns. Splitting freshness into WebApiDocsGenerator.Freshness.cs and media into its own partial keeps the main generator manageable.

⚠️ Issues to address

1. Blocking .GetAwaiter().GetResult() in WebApiDocsGenerator.Freshness.cs

var topLevel = gitClient.ShowTopLevelAsync(directory).GetAwaiter().GetResult();
var log = gitClient.RunRawAsync(...).GetAwaiter().GetResult();

Synchronously blocking on async operations risks thread-pool starvation and deadlock under ASP.NET-style synchronization contexts. If AppendGitFreshnessMetadata() cannot be made async end-to-end right now, prefer spinning up a dedicated Task.Run(() => ...).GetAwaiter().GetResult() to isolate it from any ambient sync context, or better, make the call chain async.

2. Silent null returns on git failure make diagnostics hard

if (string.IsNullOrWhiteSpace(repositoryRoot) || !Directory.Exists(repositoryRoot))
    return null; // no log, no issue emitted

If git isn't available or the working tree is unexpected, freshness silently disappears. A debug-level log or a non-fatal audit issue with a descriptive message would significantly ease troubleshooting in CI.

3. Unchecked lines[0] assumption in git log parsing

if (lines.Length < 2 || !DateTimeOffset.TryParse(lines[1], out var lastModified))
    return null;
// lines[0] is used as commitSha without validation

If git outputs a blank first line (e.g., on a fresh unborn branch), lines[0] could be empty or whitespace. Add an explicit !string.IsNullOrWhiteSpace(lines[0]) guard before consuming it as a SHA.

4. baseUrl / slugMap not null-guarded in AppendExampleMedia()

The method guards media and media.Url but passes baseUrl and slugMap directly to downstream helpers without null checks. If a caller ever passes null for these (possible when called from test harness or future callers), you'll get a NullReferenceException well inside the call stack. A ArgumentNullException.ThrowIfNull at the top of the method would make the contract explicit.

5. BOM fix only strips one leading BOM

private static string TrimLeadingByteOrderMark(string value)
{
    if (!string.IsNullOrEmpty(value) && value[0] == '\uFEFF')
        return value[1..];
    return value;
}

Extremely unlikely to matter in practice, but a malformed file with a double-BOM would still produce a \uFEFF at position 0 after stripping. TrimStart('\uFEFF') would be more defensive at essentially zero cost.

💡 Minor suggestions

• Test coverage for error paths. There are no tests for git-unavailable, git-timeout, or malformed log output in the freshness path. Even a single "git not found → freshness is null, no exception" test would prevent regressions.
• Consistency of UTF8Encoding BOM flag. Some new write sites use new UTF8Encoding(false) (no BOM), others use the static Encoding.UTF8 (which also omits BOM but less explicitly). Picking one convention throughout the new code would be cleaner.
• PowerShell execution security note. The --execute-ps-examples flag executes script content from documentation XML. The opt-in design is the right call; a brief comment in the CLI handler noting this is intentionally user-opt-in (and that it runs in the user's current process) would help future readers understand the trust boundary.

Summary

The targeted BOM/noindex fix is correct and well-tested. The blocking async calls in the freshness path (#1) and silent failure without diagnostics (#2) are the most important things to address before merging the broader feature work. The rest are lower-priority hardening suggestions.

PR Review

Overall this is a well-scoped fix with a solid regression test, plus a clean scaffolder refactor and a large batch of new features. A few observations below.

Core BOM fix (WebSiteAuditor.AssetsAndRendered.cs) ✅

The fix is correct and minimal:

private static string TrimLeadingByteOrderMark(string value)
{
    if (!string.IsNullOrEmpty(value) && value[0] == '\uFEFF')
        return value[1..];
    return value;
}

Applied consistently in both the successful and fallback decode paths of ReadFileAsUtf8. No concerns here.

Regression test (WebSiteAuditSeoMetaTests.cs) — minor inconsistency

The new test Audit_SkipsSeoMetaChecks_WhenNoIndexPageUsesUtf8Bom is correct: System.Text.Encoding.UTF8 emits BOM by default in .NET, so the file written in the test will have the BOM prefix.

However, the cleanup in the finally block uses the older inline pattern:

if (Directory.Exists(root))
    Directory.Delete(root, true);

Other tests in this PR were refactored to use TryDeleteDirectory(root). For consistency, this new test should follow the same pattern.

Scaffolder test — removed assertion with no replacement

In WebSiteScaffolderTests.cs, this assertion was removed:

- Assert.Contains("immutable commit SHA", workflow, StringComparison.OrdinalIgnoreCase);

This text was previously in the inline YAML and is now in ResolveEngineLockScriptTemplate / the generated .ps1 file. The test now asserts the .ps1 file exists but doesn't verify its content contains the SHA-format enforcement logic. Consider adding an assertion like:

var resolveScript = File.ReadAllText(Path.Combine(root, ".powerforge", "scripts", "resolve-engine-lock.ps1"));
Assert.Contains("immutable commit SHA", resolveScript, StringComparison.OrdinalIgnoreCase);

GITHUB_OUTPUT guard in the generated script

The ResolveEngineLockScriptTemplate throws if GITHUB_OUTPUT is not set:

if ([string]::IsNullOrWhiteSpace($env:GITHUB_OUTPUT)) {
  throw "GITHUB_OUTPUT is not set. This script is intended for GitHub Actions workflow steps."
}

This is a deliberate design choice that prevents accidental local runs, which is reasonable. Worth noting in documentation/comments if it isn't already, so developers who try to run it locally know why it fails.

Git freshness test — silent skip

Generate_PowerShellHelp_EmitsGitFreshnessMetadataForCommandsAndAboutTopics uses:

if (!IsGitAvailable())
    return;

A silent return on an unavailable git shows the test as passed rather than skipped, which can mask gaps in CI environments without git. xUnit's Skip attribute or Assert.Skip() would make the skip visible in test reports. Low priority, but worth considering for CI observability.

Summary

The fix itself is solid. The main actionable items are the two minor test-coverage gaps noted above.

Agent Guide (PSPublishModule / PowerForge.Web + Websites)

Last updated: 2026-03-11

This file is the "start here" context for any agent working on the PowerForge.Web engine and the three websites that use it.

Repos / Local Paths

These are the maintainer's default Windows paths (safe to assume in this workspace):

• Engine (this repo): C:\Support\GitHub\PSPublishModule

  • Web engine: PowerForge.Web\
  • Web CLI: PowerForge.Web.Cli\
  • Web docs: Docs\PowerForge.Web.*.md
  • PowerShell module: PSPublishModule\ (+ packaging under Module\)
  • Core .NET libs/CLI: PowerForge\, PowerForge.Cli\

• HtmlForgeX website: C:\Support\GitHub\HtmlForgeX\Website

  • Remote: https://github.com/EvotecIT/HtmlForgeX.git (website lives under Website\)
  • Use this location for all HtmlForgeX website work.

• IntelligenceX website: C:\Support\GitHub\IntelligenceX\Website

  • Remote: https://github.com/EvotecIT/IntelligenceX.git

• CodeGlyphX website: C:\Support\GitHub\CodeMatrix\Website

  • Remote: https://github.com/EvotecIT/CodeGlyphX.git

Portable Path Discovery (WSL/macOS/Linux)

If you're not on Windows or you don't have C:\Support\GitHub, use this layout heuristic:

• Common layout: the repos are siblings under one parent folder:
  • <root>/PSPublishModule
  • <root>/HtmlForgeX/Website
  • <root>/IntelligenceX/Website
  • <root>/CodeMatrix/Website

Practical search strategy:

1. Start at the current repo root (where this AGENTS.md lives).
2. Check .. (parent) and ../.. (grandparent) for sibling repo folders above.
3. If you're in WSL, Windows drives usually live under /mnt/c:
   • Example: /mnt/c/Support/GitHub/PSPublishModule

Recommended environment variable (makes site scripts deterministic):

• Set POWERFORGE_ROOT to the engine repo root (path to PSPublishModule).
  • Website build.ps1 scripts prefer POWERFORGE_ROOT when resolving PowerForge.Web.Cli.

What To Read First (Canonical)

1. Docs\PowerForge.Web.Roadmap.md (inventory: Have/Partial/Missing + milestones)
2. Docs\PowerForge.Web.AgentHandoff.md (high-signal handoff + commands)
3. Docs\PowerForge.Web.QualityGates.md (CI/dev contract, baselines, budgets)
4. Docs\PowerForge.Web.WebsiteStarter.md (golden path for building new sites without surprises)
5. Docs\PowerForge.Web.Parity.md (rough parity notes vs DocFX/Hugo/Astro/etc.)

Reference docs (as needed):

• Docs\PowerForge.Web.ContentSpec.md (content model + navigation)
• Docs\PowerForge.Web.Theme.md (theme anatomy + shortcodes)
• Docs\PowerForge.Web.Pipeline.md (pipeline tasks)
• Docs\PowerForge.Web.ApiDocs.md (API generator)
• Docs\PSPublishModule.ProjectBuild.md (PowerShell module build/publish pipeline)

Repo Skills (.agents/skills)

This repo ships agent skills under .agents/skills so new contributors/agents don't
need per-user global skill installs.

• Website scaffolding skill: .agents/skills/powerforge-website-starter
• Module pipeline skill: .agents/skills/powerforge-module-builder
• Library/release pipeline skill: .agents/skills/powerforge-library-builder

Working Agreements (Best Practices)

• Prefer engine fixes over theme hacks when the same issue can recur across sites.
• Keep PSPublishModule cmdlets thin:
  • parameter binding
  • ShouldProcess / prompting / PowerShell UX
  • output mapping back to PowerShell-facing contract types
• Move reusable logic into shared services first:
  • PowerForge for host-agnostic logic
  • PowerForge.PowerShell for logic that still needs PowerShell-host/runtime concepts
• Do not add new business logic to PSPublishModule\Cmdlets\ when the same behavior could be reused by PowerForge.Cli, PowerForge Studio, tests, or another C# host.
• If a public PSPublishModule result type must stay stable, keep the reusable internal model in PowerForge/PowerForge.PowerShell and map it back in PSPublishModule instead of forcing cmdlet-specific types into shared layers.
• CI/release should fail on regressions; dev should warn and summarize:
  • Verify: use baselines + failOnNewWarnings:true in CI.
  • Audit: use baselines + failOnNewIssues:true in CI.
• Prefer stable theme helpers over ad-hoc rendering:
  • Scriban: use pf.nav_links / pf.nav_actions / pf.menu_tree (avoid navigation.menus[0]).
• Commit frequently. Avoid "big bang" diffs that mix unrelated changes.

Module Layering

When touching the PowerShell module stack, prefer this boundary:

• PSPublishModule\Cmdlets\
  • PowerShell-only surface area
  • minimal orchestration
  • no reusable build/publish/install rules unless they are truly cmdlet-specific
• PSPublishModule\Services\
  • cmdlet host adapters or PowerShell-facing compatibility mappers only
• PowerForge\
  • reusable domain logic, pipelines, models, filesystem/process/network orchestration
• PowerForge.PowerShell\
  • reusable services that still depend on PowerShell-host concepts, module registration, manifest editing, or other SMA-adjacent behavior

Quick smell test before adding code to a cmdlet:

1. Could this be called from a test, CLI, Studio app, or another C# host?
2. Could two cmdlets share it?
3. Does it manipulate files, versions, dependencies, repositories, GitHub, NuGet, or build plans?

If the answer to any of those is yes, the code probably belongs in PowerForge or PowerForge.PowerShell, not directly in the cmdlet.

Stop extracting when the remaining code is only:

• PowerShell parameter binding and parameter-set branching
• ShouldProcess, WhatIf, credential prompts, and PowerShell stream routing
• host-only rendering such as Host.UI.Write*, Spectre.Console tables/rules, or pipeline-friendly WriteObject behavior
• compatibility adapters that intentionally map shared models back to stable cmdlet-facing contracts

Preferred pattern for the last 10-20%:

• extract reusable workflow, validation, planning, summary-shaping, and display-line composition into PowerForge / PowerForge.PowerShell
• keep the final host-specific rendering in the cmdlet when the rendering technology itself is PowerShell- or Spectre-specific
• avoid creating fake abstractions just to move AnsiConsole.Write, Host.UI.WriteLine, or WriteObject calls out of cmdlets

Quality Gates (Pattern)

Each website should have:

• site.json with explicit Features (docs/apiDocs/blog/search/notFound as applicable)
• pipeline.json with:
  • verify-ci step (modes:["ci"]) with baseline + failOnNewWarnings:true
  • doctor or audit step with baseline + failOnNewIssues:true
  • a simple budget like maxTotalFiles to catch output explosions early
• .powerforge/verify-baseline.json committed
• .powerforge/audit-baseline.json committed
• .gitignore ignoring _site/, _temp/, _reports/ (keep baselines committed)

Commands (Engine)

• Tests:
  • dotnet test .\PSPublishModule.sln -c Release
• File size discipline (line limit):
  • node .\Build\linecount.js . 800
  • Optional: check more file types:
    • node .\Build\linecount.js --root . --max 800 --ext .cs,.ps1,.md

Commands (PowerShell Module)

• Module lives under PSPublishModule\ (plus packaging helpers under Module\).
• The PowerForge.Web engine is .NET; the PSPublishModule is the PowerShell-facing surface.

Commands (Websites)

From a website repo folder:

• Full build:
  • .\build.ps1
• Fast dev loop:
  • .\build.ps1 -Serve -Watch -Dev
• Run CI-gated steps locally:
  • powerforge-web pipeline --config .\pipeline.json --mode ci

Baselines:

• Verify baseline:
  • powerforge-web verify --config .\site.json --baseline .\.powerforge\verify-baseline.json --baseline-generate
• Audit baseline:
  • powerforge-web audit --site-root .\_site --baseline .\.powerforge\audit-baseline.json --baseline-generate

Current State (As Of 2026-02-09)

• Engine branch: feature/web-engine-contracts contains recent quality-gate and contract hardening work.
• Website quality gates (baselines/budgets/CI-vs-dev pattern) were merged to the default branches
  of the three site repos. Avoid long-lived "quality-gates" branches; prefer small PR branches
  that get merged and deleted.

Agent Guide (PSPublishModule / PowerForge.Web + Websites)

Last updated: 2026-03-11

This file is the "start here" context for any agent working on the PowerForge.Web engine and the three websites that use it.

Repos / Local Paths

These are the maintainer's default Windows paths (safe to assume in this workspace):

• Engine (this repo): C:\Support\GitHub\PSPublishModule

  • Web engine: PowerForge.Web\
  • Web CLI: PowerForge.Web.Cli\
  • Web docs: Docs\PowerForge.Web.*.md
  • PowerShell module: PSPublishModule\ (+ packaging under Module\)
  • Core .NET libs/CLI: PowerForge\, PowerForge.Cli\

• HtmlForgeX website: C:\Support\GitHub\HtmlForgeX\Website

  • Remote: https://github.com/EvotecIT/HtmlForgeX.git (website lives under Website\)
  • Use this location for all HtmlForgeX website work.

• IntelligenceX website: C:\Support\GitHub\IntelligenceX\Website

  • Remote: https://github.com/EvotecIT/IntelligenceX.git

• CodeGlyphX website: C:\Support\GitHub\CodeMatrix\Website

  • Remote: https://github.com/EvotecIT/CodeGlyphX.git

Portable Path Discovery (WSL/macOS/Linux)

If you're not on Windows or you don't have C:\Support\GitHub, use this layout heuristic:

• Common layout: the repos are siblings under one parent folder:
  • <root>/PSPublishModule
  • <root>/HtmlForgeX/Website
  • <root>/IntelligenceX/Website
  • <root>/CodeMatrix/Website

Practical search strategy:

1. Start at the current repo root (where this AGENTS.md lives).
2. Check .. (parent) and ../.. (grandparent) for sibling repo folders above.
3. If you're in WSL, Windows drives usually live under /mnt/c:
   • Example: /mnt/c/Support/GitHub/PSPublishModule

Recommended environment variable (makes site scripts deterministic):

• Set POWERFORGE_ROOT to the engine repo root (path to PSPublishModule).
  • Website build.ps1 scripts prefer POWERFORGE_ROOT when resolving PowerForge.Web.Cli.

What To Read First (Canonical)

1. Docs\PowerForge.Web.Roadmap.md (inventory: Have/Partial/Missing + milestones)
2. Docs\PowerForge.Web.AgentHandoff.md (high-signal handoff + commands)
3. Docs\PowerForge.Web.QualityGates.md (CI/dev contract, baselines, budgets)
4. Docs\PowerForge.Web.WebsiteStarter.md (golden path for building new sites without surprises)
5. Docs\PowerForge.Web.Parity.md (rough parity notes vs DocFX/Hugo/Astro/etc.)

Reference docs (as needed):

• Docs\PowerForge.Web.ContentSpec.md (content model + navigation)
• Docs\PowerForge.Web.Theme.md (theme anatomy + shortcodes)
• Docs\PowerForge.Web.Pipeline.md (pipeline tasks)
• Docs\PowerForge.Web.ApiDocs.md (API generator)
• Docs\PSPublishModule.ProjectBuild.md (PowerShell module build/publish pipeline)

Repo Skills (.agents/skills)

This repo ships agent skills under .agents/skills so new contributors/agents don't
need per-user global skill installs.

• Website scaffolding skill: .agents/skills/powerforge-website-starter
• Module pipeline skill: .agents/skills/powerforge-module-builder
• Library/release pipeline skill: .agents/skills/powerforge-library-builder

Working Agreements (Best Practices)

• Prefer engine fixes over theme hacks when the same issue can recur across sites.
• Keep PSPublishModule cmdlets thin:
  • parameter binding
  • ShouldProcess / prompting / PowerShell UX
  • output mapping back to PowerShell-facing contract types
• Move reusable logic into shared services first:
  • PowerForge for host-agnostic logic
  • PowerForge.PowerShell for logic that still needs PowerShell-host/runtime concepts
• Do not add new business logic to PSPublishModule\Cmdlets\ when the same behavior could be reused by PowerForge.Cli, PowerForge Studio, tests, or another C# host.
• If a public PSPublishModule result type must stay stable, keep the reusable internal model in PowerForge/PowerForge.PowerShell and map it back in PSPublishModule instead of forcing cmdlet-specific types into shared layers.
• CI/release should fail on regressions; dev should warn and summarize:
  • Verify: use baselines + failOnNewWarnings:true in CI.
  • Audit: use baselines + failOnNewIssues:true in CI.
• Prefer stable theme helpers over ad-hoc rendering:
  • Scriban: use pf.nav_links / pf.nav_actions / pf.menu_tree (avoid navigation.menus[0]).
• Commit frequently. Avoid "big bang" diffs that mix unrelated changes.

Module Layering

When touching the PowerShell module stack, prefer this boundary:

• PSPublishModule\Cmdlets\
  • PowerShell-only surface area
  • minimal orchestration
  • no reusable build/publish/install rules unless they are truly cmdlet-specific
• PSPublishModule\Services\
  • cmdlet host adapters or PowerShell-facing compatibility mappers only
• PowerForge\
  • reusable domain logic, pipelines, models, filesystem/process/network orchestration
• PowerForge.PowerShell\
  • reusable services that still depend on PowerShell-host concepts, module registration, manifest editing, or other SMA-adjacent behavior

Quick smell test before adding code to a cmdlet:

1. Could this be called from a test, CLI, Studio app, or another C# host?
2. Could two cmdlets share it?
3. Does it manipulate files, versions, dependencies, repositories, GitHub, NuGet, or build plans?

If the answer to any of those is yes, the code probably belongs in PowerForge or PowerForge.PowerShell, not directly in the cmdlet.

Stop extracting when the remaining code is only:

• PowerShell parameter binding and parameter-set branching
• ShouldProcess, WhatIf, credential prompts, and PowerShell stream routing
• host-only rendering such as Host.UI.Write*, Spectre.Console tables/rules, or pipeline-friendly WriteObject behavior
• compatibility adapters that intentionally map shared models back to stable cmdlet-facing contracts

Preferred pattern for the last 10-20%:

• extract reusable workflow, validation, planning, summary-shaping, and display-line composition into PowerForge / PowerForge.PowerShell
• keep the final host-specific rendering in the cmdlet when the rendering technology itself is PowerShell- or Spectre-specific
• avoid creating fake abstractions just to move AnsiConsole.Write, Host.UI.WriteLine, or WriteObject calls out of cmdlets

Quality Gates (Pattern)

Each website should have:

• site.json with explicit Features (docs/apiDocs/blog/search/notFound as applicable)
• pipeline.json with:
  • verify-ci step (modes:["ci"]) with baseline + failOnNewWarnings:true
  • doctor or audit step with baseline + failOnNewIssues:true
  • a simple budget like maxTotalFiles to catch output explosions early
• .powerforge/verify-baseline.json committed
• .powerforge/audit-baseline.json committed
• .gitignore ignoring _site/, _temp/, _reports/ (keep baselines committed)

Commands (Engine)

• Tests:
  • dotnet test .\PSPublishModule.sln -c Release
• File size discipline (line limit):
  • node .\Build\linecount.js . 800
  • Optional: check more file types:
    • node .\Build\linecount.js --root . --max 800 --ext .cs,.ps1,.md

Commands (PowerShell Module)

• Module lives under PSPublishModule\ (plus packaging helpers under Module\).
• The PowerForge.Web engine is .NET; the PSPublishModule is the PowerShell-facing surface.

Commands (Websites)

From a website repo folder:

• Full build:
  • .\build.ps1
• Fast dev loop:
  • .\build.ps1 -Serve -Watch -Dev
• Run CI-gated steps locally:
  • powerforge-web pipeline --config .\pipeline.json --mode ci

Baselines:

• Verify baseline:
  • powerforge-web verify --config .\site.json --baseline .\.powerforge\verify-baseline.json --baseline-generate
• Audit baseline:
  • powerforge-web audit --site-root .\_site --baseline .\.powerforge\audit-baseline.json --baseline-generate

Current State (As Of 2026-02-09)

• Engine branch: feature/web-engine-contracts contains recent quality-gate and contract hardening work.
• Website quality gates (baselines/budgets/CI-vs-dev pattern) were merged to the default branches
  of the three site repos. Avoid long-lived "quality-gates" branches; prefer small PR branches
  that get merged and deleted.

PR Review

Summary

This PR addresses a real bug where BOM-prefixed HTML pages with noindex were being falsely flagged by the SEO auditor. The fix is clean and well-contained. The PR also bundles significant new features: PowerShell example validation/execution, git freshness badges, reusable CI workflows, and scoring-based example import.

Core Bug Fix: BOM stripping

The fix in ReadFileAsUtf8 / TrimLeadingByteOrderMark is correct and minimal:

• BOM strip is applied in both the strict-decode and fallback-decode paths
• value[1..] is idiomatic and efficient
• Edge case of a BOM-only file is handled safely

Minor note: Encoding.UTF8 in .NET emits a BOM by default, so the regression test (Audit_SkipsSeoMetaChecks_WhenNoIndexPageUsesUtf8Bom) correctly produces a BOM-prefixed file via File.WriteAllText(..., Encoding.UTF8). A brief inline comment noting this is intentional would help future readers.

Reusable Workflow Files

The security posture is solid -- the Resolve PowerForge engine source step enforces an immutable commit SHA at runtime, which is a meaningful supply-chain guard.

1. Maintenance workflow uses --mode ci: Both workflows pass --mode ci to the pipeline CLI. If a distinct maintenance mode exists, the maintenance workflow should use it. If ci is correct for both, a comment clarifying this is deliberate would help.

2. cancel-in-progress on the maintenance workflow: CI runs benefit from preemption on new pushes, but scheduled/manual maintenance jobs (link checks, artifact pruning, IndexNow pings) may not be safe to cancel mid-run. Consider cancel-in-progress: false for the maintenance workflow.

3. Duplicated engine-resolution step: The ~40-line PowerShell block is identical in both workflow files. A composite action could remove the duplication if this becomes a maintenance pain point.

4. Maintenance workflow lacks Playwright cache handling: The CI workflow initializes PLAYWRIGHT_BROWSERS_PATH and has a cleanup step; the maintenance workflow has neither. A comment or defensive cleanup step would help.

PowerShell Example Scoring

The GetImportedPowerShellExampleScore heuristic (filename exact-match to filename contains to path contains to noun) is well-designed. The OrderByDescending(score).ThenBy(filePath).ThenBy(lineNumber) tie-breaking gives deterministic results across runs.

Documentation

The updates in PowerForge.Web.ApiDocs.md and PowerForge.Web.Pipeline.md are thorough and well-organized. The freshness badge CSS class catalog, media node XML examples, and provenance badge documentation all clearly map to the described implementation.

Overall

The BOM fix is correct, tested, and safe to merge. The bundled features are well-documented with corresponding test coverage. Two items worth clarifying before merge:

• Is --mode ci intentional in the maintenance workflow, or should it use a maintenance-specific mode?
• Should cancel-in-progress be false on the scheduled maintenance workflow to avoid interrupting long-running jobs?

Reviewed by Claude Sonnet 4.6

test comment placeholder

DELETE - test placeholder

PR Review

The core bug fix (BOM stripping) is correct and well-tested. The broader PR also ships substantial new features. Here is detailed feedback.

Core Bug Fix - BOM Stripping (WebSiteAuditor.AssetsAndRendered.cs)

The fix is correct and clean:

• Uses range indexer [1..] correctly - no off-by-one issues
• Applied in both the happy path and the DecoderFallbackException fallback path
• Root cause: BOM (U+FEFF) at position 0 causes AngleSharp to miss the robots noindex meta tag since the document starts with a non-HTML character before the doctype declaration, explaining the misfiring SEO audit

Regression test (WebSiteAuditSeoMetaTests.cs): uses Encoding.UTF8 with File.WriteAllText, which emits the BOM preamble on .NET, so the test genuinely exercises the fix. Good coverage.

GitHub Actions Reusable Workflows

• SHA pinning is enforced for the PowerForge engine checkout - good practice.
• Inconsistency: The scaffolded website-ci.yml references the reusable workflows at @main. You enforce SHA pinning for the engine but use a floating ref for the workflow itself. A breaking change to the reusable workflow would silently affect all consumers. Consider whether @main is intentional policy or whether a version tag would be safer.
• The actions: write permission in the maintenance workflow has no inline comment explaining which step needs it - worth adding for future maintainers.
• cancel-in-progress: false for maintenance is deliberately correct but could use a brief comment.

Git Freshness (WebApiDocsGenerator.Freshness.cs)

• GetAwaiter().GetResult() is used twice per candidate file in a loop. Fine for a CLI tool, but risks deadlocks in ASP.NET/library contexts. The best-effort design (skip freshness on git failure) is good defensive coding.
• Cache granularity looks correct: repositoryRootCache by directory, freshnessCache by full file path.
• Silent misconfiguration: If GitFreshnessNewDays > GitFreshnessUpdatedDays, nothing will ever reach updated status - only new or stable. This is silently corrected with no diagnostic. Consider emitting a warning when this is detected.

Source Validation

The new sourceRoot one level above repo preflight check and urlSamples addition to the duplicate-path warning are both helpful improvements. Test coverage in both CLI and pipeline paths is thorough.

PowerShell Validation (WebApiDocsGenerator.PowerShellValidation.cs)

At 630 lines this is a large new file. The implicit enabling of validation when execution is requested is sensible - execution logically requires validation to have passed first.

Scope / PR Hygiene

The title targets a narrow bug fix, but the diff is +5,612 / -323 across 35 files (new features: git freshness, PS validation, PS media manifest, coverage thresholds, refactored workflows). This makes it harder to bisect regressions. Consider splitting features from bug fixes in future, or at minimum update the PR description to call out the full scope.

Verdict

The BOM fix is correct and regression-tested. The new features are well-structured with solid coverage. Two items to consider before merging:

1. Decide on @main vs pinned-SHA policy for reusable workflow refs in scaffolded outputs.
2. Emit a diagnostic when GitFreshnessNewDays > GitFreshnessUpdatedDays is misconfigured.

PR Review: Fix standalone doctor SEO audit for BOM noindex pages

This is a substantial, well-engineered PR that goes well beyond the stated title. The headline fix (BOM stripping for noindex SEO audit) is a legitimate bug fix with proper regression testing. The remaining ~5,500 lines introduce coherent, opt-in features: git freshness metadata, example provenance tracking, PowerShell example validation/execution, playback sidecar media, and centralized reusable CI workflows.

Overall code quality is high: public APIs have XML-doc comments, internal models are sealed, error paths are handled defensively, and test coverage is thorough.

Issues Found

Medium Severity

1. Blocking async-over-sync in WebApiDocsGenerator.Freshness.cs

Two .GetAwaiter().GetResult() calls block the thread pool on gitClient.ShowTopLevelAsync and gitClient.RunRawAsync. If the generator is ever called from an async context, this can deadlock. The GitClient API is async by design but consumed synchronously here — worth addressing as technical debt.

// Fragile pattern:
var result = gitClient.RunRawAsync(...).GetAwaiter().GetResult();

2. Silent exception swallow in AddFreshnessCandidate

The empty catch block discards all exceptions without logging. While the "best-effort" design intent is clear, this reduces debuggability. At minimum, a comment explaining the intent would help future maintainers.

3. Inconsistent cleanup in WebApiDocsGeneratorPowerShellTests.cs

Several older test methods still use the raw if (Directory.Exists(root)) Directory.Delete(root, true) pattern, whereas new tests consistently use the TryDeleteDirectory helper. On Windows, git operations can create read-only files in temp directories, causing cleanup failures with the raw pattern.

4. powerforge-website-maintenance.yml hard-codes pipeline_mode: ci

The maintenance wrapper hard-codes pipeline_mode: ci rather than a maintenance-specific mode. The inline comment explains the intent, but it means the mode enum has no practical distinguishing purpose for maintenance runs, which could be confusing to callers.

Low Severity / Style

5. TryExtractGitHubRepoName URI fabrication heuristic is fragile

Replacing template tokens with literal values to construct a parseable URI is clever but fragile. If a future token's replacement value contains /, the segment count logic could silently miscount. A comment explaining the heuristic would help.

6. RepoMismatchUrlSamples creates a new array on every mismatch hit

stats.RepoMismatchUrlSamples = stats.RepoMismatchUrlSamples
    .Append(urlSample)
    .Distinct(...)
    .Take(sampleLimit)
    .ToArray();  // allocates on every hit

For projects with many mismatched symbols, this generates GC pressure. A List<string> or HashSet<string> with a size cap would be more efficient.

7. Test helpers copy-pasted across test classes

IsGitAvailable, RunGit, and TryDeleteDirectory are duplicated in both WebApiDocsGeneratorPowerShellTests.cs and WebPipelineRunnerApiDocsPreflightTests.cs. These should be shared via a base class or a static test-utilities helper class.

8. Unnecessary HtmlEncode on internal fixed strings

In BuildFreshnessBadgeHtml, status and cssClass are internal fixed strings ("new", "updated", "type-chip-freshness") that will never contain HTML special characters. The HtmlEncode call adds noise without protection.

Security

• Git subprocess arguments use ProcessStartInfo.ArgumentList (not shell string interpolation) — correctly avoids shell injection. ✅
• Paths used in git log -- <relativePath> are validated to not traverse outside the repo root (.. prefix is rejected). ✅
• No user-controlled data rendered as raw HTML; values pass through HtmlEncode/HtmlAttributeEncode. ✅
• powerforge-website-run.yml validates engine checkout SHA against a 40/64-hex regex before use. ✅

Test Coverage

Coverage is thorough and includes:

• BOM regression (the primary fix) ✅
• Git freshness for commands and about-topics (direct API + pipeline runner) ✅
• Example origin tracking (AuthoredHelp, ImportedScript, GeneratedFallback) ✅
• PowerShell validation: syntax errors, unmatched scripts, execution failures, transcript artifacts ✅
• Playback sidecar: cast+poster staging, stale/oversized/unsupported warnings ✅
• Source URL preflight for sourceRoot placed above the repo root ✅

The IsGitAvailable() guard on freshness tests is appropriate for CI environments without git. ✅

Summary

The blocking async-over-sync pattern in the freshness implementation is the most important concern to address before this feature is used in any async host context. The other issues are minor and do not block the PR. This is a solid contribution overall.

🤖 Generated with Claude Code

Code Review

This PR combines a targeted bug fix, new CLI capability, a workflow refactor, documentation updates, and PowerShell example validation. Overall the code quality is high. A few items worth discussing:

Core Bug Fix - UTF-8 BOM Stripping (WebSiteAuditor.AssetsAndRendered.cs)

The fix is correct, minimal, and well-targeted. TrimLeadingByteOrderMark is applied in both the success path and the DecoderFallbackException catch path. The null/empty guard is correct and value[1..] is idiomatic C#. The regression test uses Encoding.UTF8 (BOM-emitting) with File.WriteAllText, which correctly exercises the fixed code path. Minor: the new test finally block still uses inline if (Directory.Exists) Directory.Delete while other tests in the same PR were refactored to TryDeleteDirectory.

Workflow Security - Bootstrap Checkout Ref (powerforge-website-run.yml)

The bootstrap checkout uses ref: github.workflow_sha to check out EvotecIT/PSPublishModule. github.workflow_sha is the SHA of the workflow file in the calling repos tree, not a stable pin of PSPublishModule. For workflow_call across repos, that SHA resolves inside the callers repo and is then used to check out PSPublishModule. This will fail or resolve to the wrong commit for any repo other than PSPublishModule itself.

The engine checkout immediately after correctly uses the immutable SHA from the lock file, but the bootstrap step (which builds the engine-lock resolve binary) does not get the same pinning guarantee.

Suggestion: Pin the bootstrap checkout to a stable tag or SHA of PSPublishModule, or add a bootstrap_ref input similar to how powerforge_ref is handled for the engine.

Scaffolded CI Workflow Pins to @main (WebSiteScaffolder.cs)

The scaffolded website-ci.yml now generates: uses: EvotecIT/PSPublishModule/.github/workflows/powerforge-website-ci.yml@main

Pinning to @main means every scaffolded site picks up whatever is on main at run time, including future breaking changes. For a build/publish tool this can cause surprise regressions for downstream consumers. Worth deciding whether living at HEAD is intentional, or whether consumers should get a version tag or SHA for reproducibility.

Engine-Lock Resolve Command (WebCliCommandHandlers.EngineLockCommands.cs)

Precedence logic is clear and correct: (1) lock file defaults, (2) explicit --repository/--ref args fill empty values, (3) env overrides with --use-env, (4) override args/env always replace.

One edge case worth a comment: if POWERFORGE_REPOSITORY or POWERFORGE_REF are set to an empty string, they pass the IsNullOrWhiteSpace guard on the fill path but the direct override assignment skips that guard. Net behavior is correct (empty env var does not clobber lock value), but a comment would make the intent explicit.

PowerShell Validation (WebApiDocsGenerator.PowerShellValidation.cs)

Timeout clamping to [5, 600] seconds is a good defensive measure. The --use-env flag gate on env variable reads is a sensible opt-in. Only executing scripts that both parse cleanly and reference documented commands is a solid safety filter before running arbitrary scripts. Surfacing asset-health warnings through coverage report fields rather than only console output is a good design decision for CI gating.

Test Infrastructure Improvements

The TryDeleteDirectory helper refactoring across WebApiDocsGeneratorPowerShellTests is a clean improvement. New engine-lock resolve tests cover both lock-file and env-override paths and verify the GitHub output key=value format.

Minor Observations

powerforge-website-maintenance.yml correctly sets cancel-in-progress: false so maintenance jobs run to completion and do not leave artifacts half-pruned. actions: write permission is scoped only to the maintenance workflow, which is appropriate. The pipeline_mode input in powerforge-website-maintenance.yml hardcodes ci (same as the CI wrapper) - if maintenance mode is eventually meant to differ from CI, this may need its own distinct value.

Verdict

The BOM fix is correct and well-tested. The two items most worth addressing before merge are the github.workflow_sha bootstrap ref (will silently misbehave for any consumer repo) and the @main pin in scaffolded workflows (affects reproducibility for downstream users). Everything else looks solid.

PR Review

Overall this is a well-structured PR. The core BOM fix is correct and the workflow refactoring cleanly DRYs up the duplicated CI/maintenance logic. A few items worth addressing:

Potential Bug / Security Concern

powerforge-website-run.yml — unquoted input interpolation in shell command

dotnet run --project ./.powerforge-engine/PowerForge.Web.Cli -- pipeline --config ./${{ inputs.pipeline_config }} --mode ${{ inputs.pipeline_mode }}

Both inputs.pipeline_config and inputs.pipeline_mode are interpolated directly into the PowerShell command string at template-expansion time, before the shell sees them. A value like config.json; Invoke-Expression(...) would be executed. Since this is workflow_call-only the callers are trusted workflows in the same repo, but defense-in-depth is still worthwhile — consider assigning the inputs to variables first:

$pipelineConfig = '${{ inputs.pipeline_config }}'
$pipelineMode   = '${{ inputs.pipeline_mode }}'
dotnet run --project ./.powerforge-engine/PowerForge.Web.Cli -- pipeline --config "./$pipelineConfig" --mode $pipelineMode

Design Issue

Maintenance workflow hardcodes pipeline_mode: ci

powerforge-website-maintenance.yml passes pipeline_mode: ci to the run workflow. The comment in the run step explains this is intentional, but it is subtle — the pipeline_mode input on the maintenance wrapper is unused and therefore misleading if someone tries to pass a different value when calling the maintenance workflow. Consider either:

• Removing the pipeline_mode input from the maintenance wrapper entirely (since it can't be changed anyway), or
• Adding an explicit comment in the inputs section of powerforge-website-maintenance.yml noting that mode is always ci.

Code Quality — SerializeExamples double-cast

if (example.Media.CapturedAtUtc is not null)
    ((Dictionary<string, object?>)payload["media"]!)["capturedAtUtc"] = ...;
if (example.Media.SourceUpdatedAtUtc is not null)
    ((Dictionary<string, object?>)payload["media"]!)["sourceUpdatedAtUtc"] = ...;

The dictionary cast is repeated (and redundant given the preceding assignment). Extracting a local variable before assigning payload["media"] is cleaner and eliminates the cast:

var mediaDict = new Dictionary<string, object?> { ... };
if (example.Media.CapturedAtUtc is not null)
    mediaDict["capturedAtUtc"] = example.Media.CapturedAtUtc.Value.ToString("O");
if (example.Media.SourceUpdatedAtUtc is not null)
    mediaDict["sourceUpdatedAtUtc"] = example.Media.SourceUpdatedAtUtc.Value.ToString("O");
payload["media"] = mediaDict;

Minor: Redundant token in checkout step

- name: Checkout website
  uses: actions/checkout@v4
  with:
    token: ${{ github.token }}

github.token is already the default for actions/checkout@v4, so this is a no-op. It is not harmful, but it may cause confusion — readers might assume a different token is needed. Consider removing it unless there is an intentional reason to be explicit.

Positive Notes

• BOM fix is correct: TrimLeadingByteOrderMark is applied in both the strict-decode path and the DecoderFallbackException fallback path — good consistency.
• Test is well-formed: Using Encoding.UTF8 (BOM-emitting) to write the fixture file correctly exercises the regression case. The test name is clear and the teardown covers both the happy and failure paths.
• SHA enforcement in engine lock: Requiring a 40/64-hex commit SHA for powerforge_ref is an excellent supply-chain hardening measure.
• Workflow DRY refactoring: The three-layer wrapper architecture (ci → run, maintenance → run) is clean. The scaffolder correctly references the versioned @main tag, keeping generated consumer workflows thin and easy to update centrally.
• SerializeExamples extraction: Eliminating six copies of the inline example serialization with a single helper is a clear improvement, especially with the addition of Origin and Media fields.

Code Review

Summary

This PR ships three distinct changes: (1) a targeted BOM-stripping bug fix in the website auditor, (2) centralised reusable GitHub Actions workflows for PowerForge.Web sites, and (3) a large batch of new API docs features (git freshness metadata, PowerShell example provenance/validation/execution, playback sidecar support, richer coverage reporting).

Bug Fix — BOM stripping (WebSiteAuditor.AssetsAndRendered.cs)

Good. The fix is minimal and correct. Trimming the BOM character after strict-decode (rather than relying on a BOM-aware overload) is the right approach. Critically, the BOM is also stripped in the error-fallback path — easy to miss, good catch.

The regression test in WebSiteAuditSeoMetaTests is well-structured and covers exactly the failing scenario described in the PR title.

GitHub Workflows (powerforge-website-*.yml)

Potential security concern — PowerShell expansion of pipeline_config:

In powerforge-website-run.yml, $pipelineConfig and $pipelineMode come directly from inputs.* and are expanded inside a PowerShell dotnet run command string. A value like pipeline.json"; Invoke-Something could be interpreted. Because these are workflow_call inputs (not user-facing workflow_dispatch), the risk surface is limited to calling workflows in the same repo — but consider using [string]’${{ inputs.pipeline_config }}’ cast syntax, which is already used correctly in the engine-resolution step above.

Floating @main reference in scaffolded workflows:

The generated templates reference the reusable workflows at @main, so consumers automatically pick up any changes merged to main. If stability matters to callers, pinning to a release tag is worth considering. If floating is intentional, documenting it explicitly would help.

Maintenance workflow has no secrets: block:

powerforge-website-maintenance.yml does not declare INDEXNOW_KEY (unlike the CI wrapper), so indexnow_key will be empty for maintenance runs. This appears intentional — a comment making it explicit would help.

cancel-in-progress: false for maintenance — correct. Scheduled maintenance jobs should run to completion. The SHA immutability check on the engine ref is excellent supply-chain hardening.

Freshness Implementation (WebApiDocsGenerator.Freshness.cs)

Sync-over-async: ShowTopLevelAsync and RunRawAsync are both called with .GetAwaiter().GetResult(). Safe in a CLI context without a SynchronizationContext, but would deadlock in ASP.NET-style environments. A brief comment noting the constraint would aid future maintainers.

Caching is correct — repositoryRootCache avoids repeated git rev-parse --show-toplevel calls; freshnessCache avoids duplicate log queries for shared source files.

Best-effort semantics are appropriate — null is returned gracefully when git is unavailable or the file is untracked.

Test Quality

Good overall. The TryDeleteDirectory helper and IsGitAvailable() guard are both improvements.

Inconsistency: Roughly 12 places in the diff still use if (Directory.Exists(root)) Directory.Delete(root, true); in existing tests. Since TryDeleteDirectory was introduced to fix Windows cleanup failures, a follow-up pass would be worthwhile.

Skipping vs. passing for git tests:
A bare return when git is unavailable reports the test as passed rather than skipped. Using xUnit’s Assert.Skip(...) would surface the skip in test reports and prevent false-positive passes on environments without git.

Coverage of new features is thorough — provenance tracking (authored/imported/generated), playback sidecars, poster-missing warnings, stale-asset detection, threshold enforcement, and validation/execution pipelines are all tested with realistic data and precise assertions.

Documentation

Comprehensive updates across ApiDocs.md, Pipeline.md, QuickStart.md, and WebsiteStarter.md. Coverage threshold naming is consistent. JSON schema updates mirror the implementation correctly.

Minor Notes

• In a couple of tests, var result = WebApiDocsGenerator.Generate(options) is assigned but result is never read — likely a dead assignment left from adding return-value assertions in other tests.
• The maintenance_mode_note boolean input exists solely to print one informational line. A YAML comment would communicate the same intent without the extra flag.

Overall

The BOM fix is solid and well-tested. The workflow refactoring meaningfully simplifies consumer workflows while adding good security hardening. The new feature batch is large but covered by thorough tests. Most items above are low-risk refinements. The pipeline_config PowerShell expansion in the run workflow is the one item worth addressing before merge.